[HW] Enhance FlattenModules pass with configurable inlining heuristics #9224

prithayan · 2025-11-11T04:34:41Z

This PR enhances the existing --hw-flatten-modules pass with configurable heuristics, hierarchical path support, and module NLA protection for fine-grained control over module inlining.

The FlattenModules pass now supports:

Configurable inlining heuristics (empty, no-outputs, single-use, small modules)
HierPath updating when inlining instances with inner symbols
Inner symbol renaming using InnerSymbolNamespace to avoid conflicts

Inlining Heuristics (7 new options)

--inline-all             (default: true)  - Inline all private modules (original behavior)
--inline-empty           (default: true)  - Inline empty modules
--inline-no-outputs      (default: true)  - Inline modules with no outputs
--inline-single-use      (default: true)  - Inline single-use modules
--inline-small           (default: true)  - Inline small modules
--small-threshold        (default: 8)     - Size threshold for small modules
--inline-with-state      (default: false) - Allow inlining stateful modules

This PR also adds new lit tests for testing these enhancements.

… pass

seldridge

LGTM.

I didn't review the inner sym / hierarchical path update logic in much detail, though. I realize this was probably the most important thing to review. However, the tests are doing the right thing!

seldridge · 2025-11-13T17:40:35Z

include/circt/Dialect/HW/Passes.td

+    Option<"smallThreshold", "small-threshold", "unsigned", "8",
+           "Maximum number of operations for a module to be considered small">,


Nit: Gating this behind --inline-small-threshold would be more obvious to me as a user.

seldridge · 2025-11-13T17:41:55Z

include/circt/Dialect/HW/Passes.td

+           "Allow inlining of modules that contain state (seq.firreg operations)">,
+    Option<"inlineAll", "inline-all", "bool", "true",
+           "Inline all private modules regardless of heuristics (default behavior)">
+  ];


Nit: Maybe prefixing these with --hw would be good. I've grown to liking having any option that is only for specific dialects is useful to indicate which dialect it works on. E.g., --dedup-classes in firtool would be better as --firrtl-dedup-classes. The same thing holds here, too.

lib/Dialect/HW/Transforms/FlattenModules.cpp

seldridge · 2025-11-13T18:54:32Z

lib/Dialect/HW/Transforms/FlattenModules.cpp

+                   InnerSymbolNamespace *ns, HWModuleOp parentModule,
+                   HWModuleOp sourceModule,
+                   DenseMap<StringAttr, StringAttr> *symMapping,
+                   mlir::AttrTypeReplacer *replacer, HierPathTable *pathsTable,
+                   hw::InnerRefAttr instanceRef)
+      : InlinerInterface(context), prefix(prefix), ns(ns),
+        parentModule(parentModule), sourceModule(sourceModule),
+        symMapping(symMapping), replacer(replacer), pathsTable(pathsTable),
+        instanceRef(instanceRef) {}


This constructor may be unnecessary if you use struct initialization. E.g., PrefixingInliner{a, b, c}. I don't know if there's any utility in adding a one-to-one constructor here as opposed to the {} one.

seldridge · 2025-11-13T18:56:41Z

lib/Dialect/HW/Transforms/FlattenModules.cpp

+  bool isEmpty = bodySize == 1;
+  bool hasNoOutputs = module.getNumOutputPorts() == 0;
+  bool hasOneUse = instanceNode->getNumUses() == 1;
+  bool hasState = !module.getOps<seq::FirRegOp>().empty();


What about memories or other kinds of state?

seldridge · 2025-11-13T19:07:22Z

test/Dialect/HW/hw-inliner-options.mlir

+hw.module @TestSingleUse(in %x: i4, out y: i4) {
+  // NONE-NEXT: hw.instance "small" @SmallModule
+  // SINGLE-NEXT: %[[V0:.+]] = comb.add %x, %x
+  // SINGLE-NEXT: hw.output %[[V0]]
+  %0 = hw.instance "small" @SmallModule(a: %x: i4) -> (b: i4)
+  hw.output %0 : i4
+}


Nit: this may want to use a @LargeModule to definitely avoid this getting inlined with the small option. I.e., there could be a bug where small inlining is turned on and it would be good if this was test was not susceptible to that.

seldridge · 2025-11-13T19:07:59Z

test/Dialect/HW/hw-inliner-options.mlir

+hw.module private @TinyModule(in %a: i4, out b: i4) {
+  %0 = comb.add %a, %a : i4
+  hw.output %0 : i4
+}


Nit: This is just @SmallModule. Though, going off of the comment above, I think you want small to be "large" and tiny to be "small".

seldridge · 2025-11-13T19:09:25Z

test/Dialect/HW/hw-inliner.mlir

+// Test that public modules are not inlined
+// CHECK-LABEL: hw.module @DontInlinePublic
+hw.module @DontInlinePublic(in %x: i4, out y: i4) {
+  // CHECK-NEXT: hw.instance "pub" @PublicModule
+  %0 = hw.instance "pub" @PublicModule(a: %x: i4) -> (b: i4)
+  hw.output %0 : i4
+}
+// CHECK: hw.module @PublicModule
+hw.module @PublicModule(in %a: i4, out b: i4) {
+  %0 = comb.add %a, %a : i4
+  hw.output %0 : i4
+}


Aside to think about: we could inline these still, just we can't delete them.

seldridge · 2025-11-13T19:33:54Z

test/Dialect/HW/hw-inliner.mlir

+  // CHECK-NOT: hw.instance
+  // CHECK-NEXT: %[[V0:.+]] = comb.add %x, %x
+  // CHECK-NEXT: %[[W0:.+]] = hw.wire %[[V0]] sym @[[SYM:wire_op]]
+  // CHECK-NEXT: sv.verbatim "ref to {{[{][{]}}0{{[}][}]}}" {symbols = [#hw.innerNameRef<@InlineWithInnerRef::@[[SYM]]>]}


Mega-nit: There is only this one verbatim and the test could avoid the {{{{{}}}}}} shenanigans with just:

CHECK-NEXT: sv.verbatim {{.*}} {symbols = ...}

Or:

CHECK-NEXT: sv.verbatim CHECK-SAME: {symbols = [#hw.innerNameRef<@InlineWithInnerRef::@[[SYM]]>]}

Ditto throughout.

seldridge · 2025-11-13T19:34:58Z

test/Dialect/HW/hw-inliner.mlir

+hw.module @InlineWithInnerRef(in %x: i4, out y: i4) {
+  // CHECK-NOT: hw.instance
+  // CHECK-NEXT: %[[V0:.+]] = comb.add %x, %x
+  // CHECK-NEXT: %[[W0:.+]] = hw.wire %[[V0]] sym @[[SYM:wire_op]]


Nit: there isn't any renaming here, so capturing this into SYM doesn't serve a purpose. The test probably does want to not capture wire_op exactly and should be .+ or an alpha-numeric capture. It's probably best to just use @wire_op directly if the expectation is this that this will be preserved exactly.

prithayan requested a review from darthscsi as a code owner November 11, 2025 04:34

prithayan marked this pull request as draft November 11, 2025 04:46

prithayan changed the title ~~[HW] Add HWInliner pass for inlining private HW modules~~ [HW] Enhance FlattenModules pass with configurable inlining heuristics Nov 11, 2025

prithayan force-pushed the dev/pbarua/hwinliner branch from 2d14fe9 to f3e4ccf Compare November 11, 2025 16:57

prithayan added 6 commits November 11, 2025 09:11

[HW] Add HWInliner pass for inlining private HW modules

52e75aa

[HW] Add HierPath support and module NLA protection to FlattenModules…

44e96ce

… pass

Update options

083d52a

Drop the HWInliner

d57857e

Drop the HWInliner

d97e314

Redo

c921ca7

prithayan force-pushed the dev/pbarua/hwinliner branch from f3e4ccf to c921ca7 Compare November 11, 2025 17:12

prithayan marked this pull request as ready for review November 12, 2025 18:19

seldridge approved these changes Nov 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[HW] Enhance FlattenModules pass with configurable inlining heuristics #9224

[HW] Enhance FlattenModules pass with configurable inlining heuristics #9224

prithayan commented Nov 11, 2025 •

edited

Loading

Uh oh!

seldridge left a comment

Uh oh!

seldridge Nov 13, 2025

Uh oh!

seldridge Nov 13, 2025

Uh oh!

Uh oh!

seldridge Nov 13, 2025

Uh oh!

seldridge Nov 13, 2025

Uh oh!

seldridge Nov 13, 2025

Uh oh!

seldridge Nov 13, 2025

Uh oh!

seldridge Nov 13, 2025

Uh oh!

seldridge Nov 13, 2025

Uh oh!

seldridge Nov 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		Option<"smallThreshold", "small-threshold", "unsigned", "8",
		"Maximum number of operations for a module to be considered small">,

[HW] Enhance FlattenModules pass with configurable inlining heuristics #9224

Are you sure you want to change the base?

[HW] Enhance FlattenModules pass with configurable inlining heuristics #9224

Conversation

prithayan commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seldridge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

prithayan commented Nov 11, 2025 •

edited

Loading